feat: add --slack-full-width for full-width Slack alerts with markdown test result tables#2133
Closed
devin-ai-integration[bot] wants to merge 9 commits intomasterfrom
Closed
Conversation
- Capitalize 'markdown' to 'Markdown' in docs - Replace invalid empty rich_text block with valid Slack Block Kit structure - Add preview validation for full-width mode - Handle markdown table truncation gracefully (row-by-row instead of mid-row) - Align description rendering between dbt and elementary test templates - Update tests to reflect valid rich_text block and exact assertions Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Contributor
|
👋 @devin-ai-integration[bot] |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds full-width Slack alert support via a new Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as CLI Monitor
participant Config as Config
participant Selector as Integration Selector
participant Builder as SlackAlertMessageBuilder
participant Slack as Slack API
User->>CLI: run monitor --slack-full-width
CLI->>Config: Config(slack_full_width=True)
Config->>Selector: evaluate integrations
Selector->>Builder: create SlackAlertMessageBuilder(full_width=True)
Builder->>Builder: prepend rich_text block
Builder->>Builder: route preview/details to main blocks
Builder->>Builder: clear attachments
Builder->>Slack: send Block Kit payload (Markdown tables)
Slack->>User: display full-width alert
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py (1)
199-205: Extract shared description block construction to prevent drift.Both template builders now have identical description rendering. A small helper would keep these paths synchronized as Slack formatting evolves.
♻️ Proposed refactor
+def _append_description_block(self, preview: list, description: Optional[str]) -> None: + description_text = description or "_No description_" + preview.append( + self.message_builder.create_text_section_block( + f"*Description*\n{description_text}" + ) + ) @@ - if DESCRIPTION_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS): - description_text = alert.test_description or "_No description_" - preview.append( - self.message_builder.create_text_section_block( - f"*Description*\n{description_text}" - ) - ) + if DESCRIPTION_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS): + self._append_description_block(preview, alert.test_description) @@ - if DESCRIPTION_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS): - description_text = alert.test_description or "_No description_" - preview.append( - self.message_builder.create_text_section_block( - f"*Description*\n{description_text}" - ) - ) + if DESCRIPTION_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS): + self._append_description_block(preview, alert.test_description)Also applies to: 365-371
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py` around lines 199 - 205, Two identical blocks build the Description Slack section; extract a small helper to centralize construction and avoid divergence. Create a private helper method (e.g., _build_description_block or build_description_block) that takes an Alert-like object or the description string, computes description_text = alert.test_description or "_No description_", and returns the result of self.message_builder.create_text_section_block(f"*Description*\n{description_text}"); then replace both occurrences (the block using DESCRIPTION_FIELD / (alert.alert_fields or DEFAULT_ALERT_FIELDS) and the identical block at the other location) to call that helper so formatting logic lives in one place.elementary/monitor/data_monitoring/alerts/integrations/slack/message_builder.py (1)
69-75: Consider skipping preview padding whenfull_width=True.Line 71 currently reuses validation that pads preview to a fixed size. In full-width mode this adds empty visible blocks and uses block budget without affecting cutoff behavior.
♻️ Proposed refactor
def add_preview_to_slack_alert( self, preview_blocks: Optional[SlackBlocksType] = None ): if not preview_blocks: return - validated_preview_blocks = self._validate_preview_blocks(preview_blocks) + validated_preview_blocks = self._validate_preview_blocks( + preview_blocks, pad_to_max=not self.full_width + ) if self.full_width: self._add_always_displayed_blocks(validated_preview_blocks) else: self._add_blocks_as_attachments(validated_preview_blocks)-@classmethod -def _validate_preview_blocks(cls, preview_blocks: Optional[SlackBlocksType] = None): +@classmethod +def _validate_preview_blocks( + cls, + preview_blocks: Optional[SlackBlocksType] = None, + pad_to_max: bool = True, +): @@ - if preview_blocks_count == SlackMessageBuilder._MAX_ALERT_PREVIEW_BLOCKS: + if ( + preview_blocks_count == SlackMessageBuilder._MAX_ALERT_PREVIEW_BLOCKS + or not pad_to_max + ): return preview_blocks🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@elementary/monitor/data_monitoring/alerts/integrations/slack/message_builder.py` around lines 69 - 75, The current flow always calls _validate_preview_blocks(preview_blocks) which pads the preview to a fixed size; when full_width=True that padding produces empty visible blocks and wastes Slack block budget. Modify message_builder.py so that when self.full_width is true you skip preview padding: either call a new validation variant (e.g., _validate_preview_blocks(preview_blocks, pad=False)) or add a parameter/flag to _validate_preview_blocks to disable padding, then pass the unpadded validated_preview_blocks into _add_always_displayed_blocks; keep existing padded validation for the non-full_width branch used by _add_blocks_as_attachments. Ensure references to _validate_preview_blocks, _add_always_displayed_blocks, and _add_blocks_as_attachments are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py`:
- Around line 225-237: The Slack message can exceed Slack limits (50 blocks and
3000 chars per mrkdwn section) when embedding large code-fenced tables; update
the logic around table_max_length / list_of_dicts_to_markdown_table to enforce
truncation so the resulting test_rows_sample_table plus the surrounding "```"
stays <= 3000 chars, and ensure you do not create more than 50 blocks when
building with self.message_builder.create_text_section_block (consider
collapsing or omitting less critical sections). Specifically, adjust the
reserve-for-fence calculation (currently using SectionBlock.text_max_length -
6), enforce a hard cap on test_rows_sample_table length before wrapping it in a
code fence, and add a guard in the message-building flow to skip or summarize
this sample when total blocks would exceed Slack's 50-block limit.
In `@elementary/utils/json_utils.py`:
- Around line 142-161: The fallback can still exceed max_length when a
single_row_table is too long; update the fallback logic after producing
single_row_table (from processed_data[:1]) to check its length against
effective_max and, if necessary, truncate the table string to effective_max
characters and append truncation_note so the final return never exceeds
max_length; use the existing truncation_note and effective_max variables and
return the truncated single_row_table + truncation_note when trimming is
required.
---
Nitpick comments:
In
`@elementary/monitor/data_monitoring/alerts/integrations/slack/message_builder.py`:
- Around line 69-75: The current flow always calls
_validate_preview_blocks(preview_blocks) which pads the preview to a fixed size;
when full_width=True that padding produces empty visible blocks and wastes Slack
block budget. Modify message_builder.py so that when self.full_width is true you
skip preview padding: either call a new validation variant (e.g.,
_validate_preview_blocks(preview_blocks, pad=False)) or add a parameter/flag to
_validate_preview_blocks to disable padding, then pass the unpadded
validated_preview_blocks into _add_always_displayed_blocks; keep existing padded
validation for the non-full_width branch used by _add_blocks_as_attachments.
Ensure references to _validate_preview_blocks, _add_always_displayed_blocks, and
_add_blocks_as_attachments are updated accordingly.
In `@elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py`:
- Around line 199-205: Two identical blocks build the Description Slack section;
extract a small helper to centralize construction and avoid divergence. Create a
private helper method (e.g., _build_description_block or
build_description_block) that takes an Alert-like object or the description
string, computes description_text = alert.test_description or "_No
description_", and returns the result of
self.message_builder.create_text_section_block(f"*Description*\n{description_text}");
then replace both occurrences (the block using DESCRIPTION_FIELD /
(alert.alert_fields or DEFAULT_ALERT_FIELDS) and the identical block at the
other location) to call that helper so formatting logic lives in one place.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
docs/oss/deployment-and-configuration/slack.mdxdocs/oss/guides/alerts/send-slack-alerts.mdxelementary/config/config.pyelementary/messages/formats/block_kit.pyelementary/messages/formats/markdown.pyelementary/messages/formats/text.pyelementary/monitor/cli.pyelementary/monitor/data_monitoring/alerts/integrations/integrations.pyelementary/monitor/data_monitoring/alerts/integrations/slack/message_builder.pyelementary/monitor/data_monitoring/alerts/integrations/slack/slack.pyelementary/utils/json_utils.pytests/unit/monitor/data_monitoring/alerts/integrations/slack/test_slack_alert_message_builder.pytests/unit/utils/test_json_utils.py
Comment on lines
+225
to
237
| # Reserve space for the code fence (``` prefix and ``` suffix = 6 chars) | ||
| table_max_length = SectionBlock.text_max_length - 6 | ||
| test_rows_sample_table = list_of_dicts_to_markdown_table( | ||
| alert.test_rows_sample, max_length=table_max_length | ||
| ) | ||
| result.extend( | ||
| [ | ||
| self.message_builder.create_context_block( | ||
| ["*Test results sample*"] | ||
| self.message_builder.create_text_section_block( | ||
| f"{ICON_TO_UNICODE[Icon.MAGNIFYING_GLASS]} *Test results sample*" | ||
| ), | ||
| self.message_builder.create_text_section_block( | ||
| f"```{alert.test_rows_sample}```" | ||
| f"```{test_rows_sample_table}```" | ||
| ), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Slack API maximum blocks per message limit documentation
💡 Result:
Slack’s Block Kit documentation states that a message’s blocks array can include up to 50 blocks per message (and up to 100 blocks in modals or Home tabs). [1]
Docs: Slack Developer Docs → “Blocks” (Block Kit reference). [1]
Sources
[1] Slack Developer Docs — “Blocks” (Block Kit reference): https://docs.slack.dev/reference/block-kit/blocks/
🌐 Web query:
Slack mrkdwn section text field maximum character length limit
💡 Result:
In Slack Block Kit, a section block’s text field (a mrkdwn text object) has a maximum length of 3000 characters. [1][2]
If you’re using section.fields (an array of text objects), each field’s text max is 2000 characters (max 10 fields). [1]
Run end-to-end Slack integration test with large sample payloads before release.
This path combines multiple section blocks with code-fenced sample tables. Slack enforces a 50-block limit per message and 3000-character limit per mrkdwn section text field. Validate that near-limit payloads (with large test result samples) do not trigger invalid_blocks or msg_too_long errors against the actual Slack API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py` around
lines 225 - 237, The Slack message can exceed Slack limits (50 blocks and 3000
chars per mrkdwn section) when embedding large code-fenced tables; update the
logic around table_max_length / list_of_dicts_to_markdown_table to enforce
truncation so the resulting test_rows_sample_table plus the surrounding "```"
stays <= 3000 chars, and ensure you do not create more than 50 blocks when
building with self.message_builder.create_text_section_block (consider
collapsing or omitting less critical sections). Specifically, adjust the
reserve-for-fence calculation (currently using SectionBlock.text_max_length -
6), enforce a hard cap on test_rows_sample_table length before wrapping it in a
code fence, and add a guard in the message-building flow to skip or summarize
this sample when total blocks would exceed Slack's 50-block limit.
Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat: add --slack-full-width for full-width Slack alerts with Markdown test result tables
Summary
Adds a
--slack-full-widthCLI flag that renders Slack alerts at full message width using Block Kit main-body blocks (no attachments) and displays test result samples as Markdown tables. This addresses readability issues when test results contain many columns or long values (#1079).Key implementation details:
rich_textsentinel block is injected at the start to force Slack into full-width layout; attachments are clearedlist_of_dicts_to_markdown_table()converts test result samples to GitHub-flavored Markdown tables with graceful row-by-row truncation (avoids cutting mid-row at the 3000-char Slack limit)SlackAlertMessageBuildergains afull_widthflag that routes preview/detail blocks to main blocks instead of attachments_get_dbt_test_templateand_get_elementary_test_templatenow use a singletext_section_blockfor descriptions (was two blocks: header + context)disable_numparse=Trueadded to alltabulatecalls to preserve numeric formattingReview & Testing Checklist for Human
rich_textspace-character block renders acceptably in Slack. The{"type": "text", "text": " "}is valid per API but may show as a blank line. Test with--slack-full-widthin a real Slack channel._get_elementary_test_template(andcreate_context_block→create_text_section_blockswaps for "Result message") changes the appearance of all elementary test alerts. Verify this is intentional.full_width). Verify this is desired.column_nameis falsy, the column section is now entirely omitted instead of showing "No column". Applies to all alerts, not just full-width.config.is_slack_workflow or config.slack_full_widthforcesSlackIntegrationeven for webhook users. Verify this is correct.Notes
code-qualityCI passed. Full end-to-end Slack rendering not tested.Summary by CodeRabbit
New Features
Improvements
Documentation
Tests